feat(desktop): move persisted secrets from JSON into keyring via gen-ref protocol - #5486
Draft
wpfleger96 wants to merge 32 commits into
Draft
feat(desktop): move persisted secrets from JSON into keyring via gen-ref protocol#5486wpfleger96 wants to merge 32 commits into
wpfleger96 wants to merge 32 commits into
Conversation
…ref protocol
All three secret tiers (global env vars, per-agent env/auth_tag/
provider_config, definition env vars) now move from plaintext JSON
into the existing SecretStore keyring blob using an immutable
generation-reference protocol:
- Each write creates a new immutable generation entry under a UUID-
keyed coordinate (e.g. agent:<pubkey>:env:<gen>). The stripped JSON
carries a non-secret *_ref field; the atomic JSON write is THE commit
point.
- Crash before JSON commit: old ref stays authoritative, orphaned gen
is swept by GC. Crash after: new ref is authoritative, old gen swept.
- Empty vs unavailable distinguished by ref presence: no ref = field
intentionally empty (agent runs); ref present but entry missing =
unavailable, fail closed (nsec-style refusal).
- Inline precedence: on keyring write failure (Windows TooLong /
backend error) value stays inline in 0o600 JSON with a named warning
and the ref is cleared. Inline is authoritative over any keyring state
for that boot; extraction retries next boot.
- Two-cycle GC: sweep 1 marks unreferenced generations as candidates in
the blob; sweep 2 (next verified boot) deletes still-unreferenced
candidates. Saves cancel their generation's candidacy before the JSON
commit. GC is a no-op when either JSON store is absent, unreadable,
or changes between reference collection and blob mutation.
- Boot migration runs at the end of run_boot_migrations_inner (after
materialize_agent_runtimes) so raw-JSON migrations see inline values.
- OSS keyringless builds are unchanged (inline 0o600 JSON).
Coordinates:
global:env:<gen>
agent:<pubkey>:env:<gen>
agent:<pubkey>:auth_tag:<gen>
agent:<pubkey>:provider_config:<gen> (entire BackendKind::Provider.config)
definition:<id>:env:<gen>
Files changed:
secret_store.rs — add remove_keys() for batch blob deletion
managed_agents/mod.rs — expose secret_projection module
managed_agents/secret_projection.rs — new: full gen-ref protocol impl + tests
managed_agents/types.rs — add auth_tag_ref, env_vars_ref, provider_config_ref
managed_agents/global_config — add env_vars_ref, hydrate-on-load/strip-on-save
managed_agents/storage.rs — hydrate/strip seam, migration helpers
migration.rs — boot migration + two-cycle GC
commands/agents.rs — new ref fields initialised to None
commands/personas/snapshot/import.rs — same
commands/team_snapshot.rs — same
Deferred (noted in PR body, not a separate issue): existing nsec blob
(~2.8 KB) already exceeds the Windows 2,560-byte cap — pre-existing,
same accepted-risk class as the overflow path added here.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
marked this pull request as draft
August 10, 2026 16:00
… tests Paul-review deltas on PR #5486: - local_setup now reflects secrets_unavailable: both status_for_with and unkeyable_failed_status set local_setup = false when a record has a dangling keyring ref. Previously, runtimes like claude/codex (which pass agent_readiness without API-key checks) would show local_setup = true even when spawn was blocked by spawn_key_refusal. The flag was already wired to spawn refusal; this closes the status gap. - Legacy Sprout app-data agents dir cleanup: migrate_legacy_app_data_dir copies the old xyz.block.sprout.app agents/ into Buzz but leaves the source. cleanup_secret_artifacts now also runs on the legacy source dir after a verified extraction boot, scrubbing backups and deleting .invalid/temp artifacts from the Sprout-era plaintext store. - Phase 2 artifact cleanup tests: add filesystem-backed tests for atomic-write temp deletion, .invalid deletion, parseable-backup scrub, symlink escape (unix-only), and deletion-failure tolerance. These cover the cleanup_secret_artifacts inventory that the spec requires. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…sal + dev secrets migration Commit the three items that were in working-tree state but not pushed: - types.rs: add secrets_unavailable: bool field (transient, #[serde(skip)]) to ManagedAgentRecord with doc comment explaining load-time semantics - secret_seam.rs: set record.secrets_unavailable = true when hydration errors are present, making the setter actually wire unavailability into the record - storage.rs: extend spawn_key_refusal to check secrets_unavailable (in addition to empty nsec), refuse spawn with a named error message; update comments at both load_managed_agents callers to reflect that unavailability is now set directly on the record and consulted by spawn_key_refusal - storage.rs: add migrate_agent_secrets_to_dev_service (v2 marker, separately versioned from _dev_migration_v1) covering canonical dev → prod and scoped dev → canonical dev, with conflict detection, global:env ref check, and collect_global_env_refs helper - migration.rs: wire migrate_agent_secrets_to_dev_service into the boot path (cfg(debug_assertions), after migrate_inline_secrets_to_keyring) - secret_projection.rs: add test_cancel_before_mark_ordering_protects_in_flight_gen covering the cancel-before-mark scenario Paul flagged (step 4 safe: a marked- then-referenced gen survives the next boot's delete phase) - All struct literal sites updated with secrets_unavailable: false Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… ratchet Complete the module extraction and field wiring left in working-tree state: - Wire dev_service_migration and secret_projection_tests as modules - Extract dev keyring secrets migration into dev_service_migration.rs and secret_projection tests into secret_projection_tests.rs so both parent files stay under the 1000-line desktop ratchet - Reclaim ratchet lines in the six over-cap files touched by the secrets_unavailable field additions (types.rs, migration.rs, agents.rs, import.rs, discovery/tests.rs, readiness.rs) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ker, cleanup gate F1 CRITICAL: Remove all eager old-generation deletion from secret_seam.rs and global_config/mod.rs. Old-gen retirement moves entirely to two-cycle GC. Add 5 deterministic JSON-write-failure tests proving old generation still hydrates after a failed atomic write (instance env/auth/provider, definition env, global env). F5b (collect_live_refs validates): Reject malformed/duplicate coordinates and inline+ref conflicts — any such condition makes the sweep a no-op. Add 12 validation tests. F5a (GC exclusion, not just time): Wrap extraction + global save + GC under managed_agents_store_lock so final JSON read and remove_batch are indivisible against a concurrent save. Add 3 synchronized interleaving tests. F3 (fail-closed global + definition tiers): - Global: load_global_agent_config() returns Err on missing/corrupt ref; callers in runtime.rs/runtime_commands.rs now surface global_unavailable flag instead of unwrap_or_default(); spawn_agent_child uses ? to abort. - Definition: AgentDefinition gains secrets_unavailable field (#[serde(skip)]); hydrate_all_secrets_for_records sets it on definition records; spawn gate in runtime.rs refuses linked instances when definition is unavailable; status_for_with checks definition_unavailable tier; agents_deploy.rs checks it too. Add definition_tier_tests.rs with 3 tests. F4 (dev migration marker only on clean completion): Extract pure decision core plan_dev_secrets_migration(); marker withheld when conflict_count > 0; partial progress (non-conflicting keys) still written for next boot; log emitted. F2 (cleanup authorization + legacy live file): Replace bare agent_secret_store_pub().is_some() gate with extraction_verified() that reloads both stores and checks no secrets_unavailable flags set after hydration. Add scrub_legacy_live_file() to explicitly scrub/delete legacy managed-agents.json after verified extraction. Add 4 tests. AgentDefinition derives Default to support test helper construction. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ict hydration refusal, GC live-ref + cross-process lock Close the five pass-2 blocking findings on the keyring secret-projection PR. F3: add require_effective_secrets_available — a single fail-closed gate (strict global loader, instance secrets_unavailable, linked-definition secrets_unavailable) consumed by every side-effecting path that was degrading silently: get_agent_models, mint_agent_card, the update_managed_agent rename, persona name/avatar propagation signing, and set_global_agent_config (which now refuses to overwrite a committed global whose ref cannot load rather than unwrap_or_default over it). F4: a dev-migration value conflict now writes a conflict:<coord> marker into the keyring blob. load_secret fails closed whenever a coordinate carries one, so hydration sets secrets_unavailable and every downstream gate refuses — making the conflicted value genuinely unavailable for the whole retry window instead of merely withholding the completion marker. A resolved conflict clears its marker. F5b: collect_live_refs now returns full expected blob coordinates alongside gen ids, and both GC sweeps no-op when any committed live reference is missing from the blob — so a dangling live ref can no longer let GC delete an older unreferenced generation that may be the only recoverable payload. F5a: add a cross-process transaction lock (a second advisory lockfile, distinct from the per-op mutate_blob lock) held from generation write through the JSON commit on every save path, and across the live-ref read through remove_batch in GC — closing the two-process interleave where one process could delete a generation another had written but not yet committed. Snapshot export: materialize_snapshot_bytes propagates the global loader Err instead of unwrap_or_default, refusing export/send rather than silently shipping empty inherited runtime/provider/model defaults. agent_models.rs, card.rs, and secret_store.rs are kept under the desktop file-size ratchet via #[path] sibling extraction (naming helpers, env-layer key resolution, and the secret_store test module respectively). Adds unit tests for the gate (all three tiers + precedence), conflict hydration→spawn-refusal, GC missing-coordinate freeze (both sweeps + positive bound), independent-participant txn-lock exclusion, and snapshot degradation refusal. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ocess txn lock Close the r4 blocking findings on the keyring secret-projection PR: - Fail-closed load: a conflict-marker read that returns Err is now treated as unavailable, not fall-through. load_blob caches successful reads but never errors, so a transient marker-read failure could be followed by a cached-success value read that hydrates a known-conflicted credential. - Conflict-marker cleanup only on proven resolution: the dev-migration planner clears a conflict:<coord> marker only when source and destination converge OR the coordinate is proven no longer live in canonical JSON. A disappearing source while the destination value is still live retains the marker (fail closed). - Cross-process transaction lock (Race 1): the lock now spans the other-half read through generation writes to the JSON commit in both save paths, so two Desktop processes cannot interleave a cross-half overwrite. Keyed by the symlink-resolved canonical store directory inode, not the keyring service, so processes sharing one JSON file always contend on one lock. - Transaction lockfile out of /tmp: the lock target is the store directory inode in the owner's app-data tree, immune to the tmp-cleaner unlink/recreate split that let two processes both "hold" a /tmp lockfile. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Bring in main's runtime.rs mesh acp_model wire translation so local checks and CI both run on the merged tree. Clean auto-merge; the PR's fail-closed spawn gating and main's model translation touch disjoint regions of spawn_agent_child. * origin/main: (24 commits) Improve desktop search scoping (#5306) Add glass appearance and cohesive settings (#5478) Add Send to channel for thread messages (#5305) Fix macOS attachment picker lifecycle and allow inert HTML downloads (#5569) fix(desktop): preserve fresh channel timelines (#5577) fix(desktop): suppress fresh focus-return refetches for channels and home-feed (#5535) chore: mesh upgrade, clean up legacy special case code, simplify model selection for mesh (#5289) fix(desktop): preserve theme when opening communities (#5266) fix(link-preview): resolve YouTube videos through oEmbed (#5520) fix(buzz-agent): harden Databricks OAuth token cache and callback (#5534) fix(link-preview): reliably render previews sent right after they resolve (#5245) fix(link-preview): restore Buzz entity link cards (#5494) chore(release): release Buzz Desktop version 0.5.9 (#5521) feat(cli): add --visibility flag to channels update (#5119) Polish desktop onboarding flow (#5310) fix(desktop): quiesce renderer polling while hidden (#3677) (#5490) fix(channels): restore member invitations to private channels (#5493) perf(ci): experiment with sccache for relay builds (#5224) fix(desktop): bound nine unbounded localStorage stores (#5454) feat(desktop): time-based sweep for stale localStorage caches (#5453) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Merge-collision ratchet relief. Merging current main pushed managed_agents/runtime.rs to 1012 split-count (>1000 limit): the PR's fail-closed spawn gating (+16) and main's mesh acp_model translation (+12) landed in disjoint regions of one file. Move the self-contained persona_drift_state helper verbatim to a new runtime/drift.rs sibling (the file's existing plain-submodule pattern) and re-export it, dropping runtime.rs to 986. No logic change: the fn body is identical; only visibility (pub(crate)) and an import replacing the inline fully-qualified type path differ. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…yring secret refs The boot migration ran already-projected records (empty inline, live ref) through the strip-on-save seam, which reads an empty inline field as a user clear and drops the ref — silently wiping every committed secret ref on the second launch, then GC deletes the orphaned generations. And save_managed_agents re-read the definition half through the hydrating loader, so every instance-side save re-inlined definition secrets back into plaintext JSON and froze GC on the inline+ref conflict. Give the migration its own field-granular transition (migrate_inline_field -> FieldMigration) that only projects a non-empty inline value and never clears a ref it did not write; expose it as a pub(crate) seam so the custom-harness migration shares one W1-safe semantic. Re-read the definition half RAW under the txn lock and propagate a parse error with ? instead of unwrap_or_default(), so a malformed store fails the save rather than deleting every definition. Broaden the backup recognizer structurally to catch this repo's own pre-backfill.bak and pre-team-suffix-strip.bak producers. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Pull W8 forward: main advanced 18 commits and touched two files this branch also touches (managed_agents/runtime.rs, commands/agent_models_tests.rs), so the branch-skew guard blocked pushes. Merge (not rebase) to preserve the reviewed r5 SHAs; both overlaps auto-resolved cleanly (runtime.rs keeps main's idle_pool_sleep env line alongside this branch's persona_drift extraction; agent_models_tests.rs keeps both sides' record fields and new tests). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
storage.rs and storage_tests.rs crossed the desktop 1000-line file-size ratchet after the keyring merge. Lift the log/receipt/PID domain (log-path resolution, rotation, install logs, runtime receipts, PID files, log-tail error extraction) into a storage/logs.rs submodule re-exported via `pub use logs::*`, and move its tests to storage/logs_tests.rs. Pure movement: each item keeps its original visibility so all caller paths resolve unchanged, and the workspace test count is identical before and after (2623 passed / 0 failed / 15 ignored). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The service-keyed /tmp blob lockfile could be unlinked by a tmp cleaner while a holder kept its flock; a recreate under the same pathname is a fresh inode a second process can lock in parallel, splitting mutual exclusion across two inodes. Recheck (dev, ino) after the lock is granted and re-acquire against the live pathname on a mismatch, bounded by MAX_BLOB_LOCK_REACQUIRE so a pathname churned faster than we can lock fails loudly instead of spinning. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…n reuse The seam wrote each secret field with its own mutate_blob call and minted a fresh generation every save, so a metadata-only save churned the keyring blob and left GC-eligible orphan generations behind. write_secrets_batched reuses the live generation when a field's bytes are unchanged (no write, no new UUID, no GC churn) and commits every changed field in a single store_batch_verified mutation. Per-field cancel_gc_candidacy is dropped on this path: under the txn lock GC cannot run, a fresh UUID gen was never observed by a sweep, and a reused gen is a live ref sweeps skip, so no candidate marker can exist at save time. Verification bypasses the cache via verify_stored_raw so a backend that acks a write it did not persist is still caught. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…xn lock Identity and agent secrets share one SecretStore blob, but the identity persist path took no transaction lock, so an identity keyring write could interleave with a concurrent agent save/GC projection transaction on the same blob. Acquire the secret txn lock at the AppHandle-bearing entry points (import_identity, persist_current_identity, pairing recover), held across the persist span only. Lock order is identity_mutation -> txn -> blob on identity paths and txn -> blob on save/GC paths, so no path acquires them in opposing order. The boot path is left to single-instance serialization (lib.rs single-instance plugin). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…-projection * origin/main: Harden shared agent instruction review (#4220) chore(release): release Buzz Desktop version 0.5.11 (#5714) feat(acp): report standard adapter usage (#4950) fix(mobile): settle hydrated threads on latest reply (#4702) perf(desktop): persist channel snapshot hash (#5684) fix(agent): raise output limit and allow 3 recoveries (#5475) fix(desktop): defer foreground resume work (#5696) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> # Conflicts: # desktop/src-tauri/src/commands/personas/update.rs # desktop/src-tauri/src/managed_agents/mod.rs
custom_harnesses.rs is about to grow with keyring env projection, so lift its unit tests into a #[path]-included custom_harnesses_tests.rs sibling to stay under the desktop 1000-line file-size ratchet. Pure movement: production code is byte-identical, and the 43 test functions move unchanged into the sibling module. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
migration.rs is at the desktop 1000-line file-size ratchet's frozen base (1408, pinned on origin/main), so the custom-harness boot-migration wiring cannot add its module registration without tripping the ratchet. Lift the self-contained databricks V1->V2 reconcile pair (reconcile_databricks_v1_to_v2 + its _in_file helper) into a migration/databricks_reconcile.rs submodule and move its tests to the sibling databricks_reconcile_tests.rs, reclaiming headroom. Pure movement: function bodies are byte-identical; only patch_json_records/canonical_dev_data_dir gain a super:: qualifier, the entry point narrows pub -> pub(super) (its sole caller is the boot call), and the test file's test_support import re-roots to crate::migration::test_support. All 11 databricks reconcile tests stay green. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Custom harness definitions carried their `env` map (which can hold provider secrets like ANTHROPIC_API_KEY) in plaintext on disk. Mirror the agent-store secret seam: add an `env_ref` generation pointer, strip `env` into the OS keyring under `harness:<id>:env:<gen>` on save, hydrate it back on load, and write the stripped JSON 0o600. A boot migration lifts pre-existing inline env, verifies each projected generation reads back, then scrubs plaintext-bearing backup/temp artifacts. Keyless builds and keyring outages keep env inline as an authoritative fallback. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The save-path .bak comments called a leaked backup unconditionally harmless. That holds only when the harness env was projected into the keyring; in the keyring-unavailable/keyless fallback the env stays inline, so the .bak carries plaintext secrets. State the real contract and point at the boot migration that scrubs *.json.bak for exactly that case. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The definition tier of the fail-closed spawn gate — refuse to launch or deploy a linked instance whose definition's env_vars ref could not be hydrated from the keyring — was open-coded identically at four sites (spawn, two status rows, deploy) and reimplemented again in tests. Extract `unavailable_definition_id` next to its sibling `spawn_key_refusal` and route every site through it. It returns the offending definition id so the spawn and deploy paths can name it in the refusal message without a second lookup; status rows use `.is_some()`. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…seam tests The r5 review flagged several fail-closed seams as under- or falsely tested. This makes the coverage and comments truthful at each: - global-config save and snapshot export each extract an AppHandle-free seam (refuse_save_on_unavailable_current, resolve_snapshot_global) so the missing/unreadable-ref refusal is unit-testable; both map only the Err arm and pass Ok through unchanged. - runtime_commands readiness tests drop two assertion-free let _ = status bodies for real local_setup assertions, with a positive control that proves the negatives are caused by the definition-unavailable gate. - a concurrent instance/definition save interleave test pins that neither half is lost or re-inlined under the shared txn lock; save_agent_ definitions_at is split out as the path-based seam it drives. - dev_service_migration, secret_store, and runtime comments corrected to match the code (marker-clear proof conditions, completion vs per- coordinate markers, Windows named mutex, lone degraded-empty consumer). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The interleave/txn-lock regressions drove `transaction_lock_at` directly in the test, not the production lock acquisition, so deleting the real lock in `save_managed_agents`/`save_agent_definitions` or the identity persist path left both tests green. The lock primitive was proven; the production wiring was not (Thufir r5 IMPORTANT #1). Extract AppHandle-free entry points that own lock -> mutation as one unit: `save_managed_agents_locked_at`/`save_agent_definitions_locked_at` in storage, and `persist_identity_locked` (in commands/identity, the command-orchestration layer) for the three identity-persist sites. All production wrappers route through them. Identity persist contends on the same store-directory inode every agent save takes via `secret_txn_lock_dir`, so identity/agent secrets on the shared blob mutually exclude. The interleave tests now drive those exact seams via a store-injected barrier that pauses mid-mutation inside the lock span; a non-blocking flock(LOCK_NB) probe proves EWOULDBLOCK exclusion. Deleting either storage acquisition or the identity acquisition now turns the matching regression red. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
A persisted harness env_ref whose keyring generation is missing, unreadable, or malformed hydrated to an empty env, the definition still reached the loaded registry and spawn descriptor, and a subsequent metadata save read the empty partially-hydrated view as an intentional clear and erased the ref — turning a keyring outage into permanent pointer loss. Add a runtime-only env_unavailable marker (#[serde(skip)], reconstructed on load) mirroring the agent tier's secrets_unavailable. hydrate_harness_env sets it when a present env_ref cannot be resolved. Fail closed at all four harness seams — spawn, deploy, model discovery, and both status local_setup sites — via unavailable_harness_id. On save, persisted_unavailable_env_ref re-reads the on-disk record so an empty-env save over an unavailable record preserves the raw ref instead of clearing it; a genuine clear of a healthy record still clears. A rename of an unavailable record is refused: the ref key embeds the id, so it can neither be re-read under the new id nor carried forward safely. Also correct the harness_env_key comment: unreclaimed generations accrete (no harness-namespace GC), they are not bounded. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Automatic restart flows (post-install adapter bounce, global-config change) derived eligibility purely from resolve_effective_agent_env + agent_readiness. An unavailable secret tier hydrates to an empty env that is indistinguishable from an intentionally-empty one, so those flows could stop a live/setup process and only discover the refusal at respawn (FailedAfterStop). Add effective_secrets_unavailable(record, personas) — ORs the three registry-derivable tiers (instance secrets_unavailable, definition, harness) — and thread it through both eligibility predicates and the under-lock recheck in each flow, refusing before any stop. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The restart-eligibility gate pushed agent_discovery.rs over the 1000+ desktop file-size ratchet ceiling (1819 -> 1849). Move the test module to a sibling agent_discovery_tests.rs included via #[path], matching the runtime_commands_tests.rs convention. Pure relocation, no behavior change. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… seams The pass-2 secret-availability gate closed the stop-then-fail-at-respawn class for the record-derivable tiers (instance, definition, harness) but left two gaps: 1. Global tier: both post-install restart checks loaded the global config with unwrap_or_default() while spawn_agent_child reloads it strictly, so a restart authorized while the global ref was unavailable — even for an unrelated secret — would stop a live process only to hit the spawn refusal (FailedAfterStop). The global-config-save flow's under-lock recheck reused phase-1 snapshots and never re-attested the committed global ref before the stop. 2. The four production restart-gate call sites consulted effective_secrets_unavailable at the call site, so mutating any single call did not fail a test — only the pure predicate was bound. Add refuse_restart_on_unavailable_global (mirrors refuse_save_on_unavailable_current) and route the strict global load through it at both post-install sites and the global-config under-lock recheck. Consolidate each flow's secret consultation into AppHandle-free composed seams (should_restart_after_install_for, refuse_restart_on_unavailable_secrets, should_restart_on_config_change_for) so the consultation lives in one testable place, and bind each with record-driven regressions. Global availability stays a separate strict Result gate rather than a fourth arm of effective_secrets_unavailable — it is not derivable from a record. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…-projection * origin/main: test: add deterministic desktop release smoke (#5699) fix(channels): return complete member rosters (#5765) feat(desktop): add Inbox message delete action (#5779) fix(desktop): enforce agent mention authorization at send boundaries (#5681) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…module The restart-eligibility gate (effective_secrets_unavailable) covered the instance/definition/harness tiers but not the global tier. A setup-mode agent could be stopped by an auto-restart while the committed global ref was unhydratable, then hit the strict global reload at spawn, refusing respawn after the live process was already stopped (FailedAfterStop). Extract the four restart flows into an AppHandle-free restart_ops module. Each op takes the global-config loader as a closure it invokes itself, so the strict-global re-read and load-before-stop ordering are op-owned and stop-spy-testable. The two pre-scan selectors own the strict-global gate (unreadable global yields zero candidates); the two under-lock authorizers re-read the committed global before the stop. Production shells in agent_discovery.rs and global_agent_config.rs become thin wiring. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…-projection * origin/main: feat(desktop): one relative date ladder across chat and the Inbox (#3769) fix(desktop): amortize observer journal eviction with a low-water mark (#5808) Unify agent profile content (#5788) Standardize settings section layout (#5855) fix(desktop): share one timer across same-interval useNow consumers (#5861) Clarify immediate spoken huddle replies (#5863) Scope desktop presence subscriptions to active demand (#5830) Polish mobile profiles, DMs, and sheets (#5401) fix(huddle): stop 20 Hz speaker-level churn from re-rendering the whole app (#5825) Fix channel list scroll interruption (#5815) fix(desktop): match compact link preview thumbnail corners to card shell (#5711) feat(huddle): cut voice-turn time-to-first-audio from ~1.0 s to ~0.35 s (env-gated latency levers) (#5671) Speed up initial direct messages (#5658) Polish glass Huddle tray behavior (#5590) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Provider API keys and other secret-shaped values set as agent env vars were persisted plaintext in
global-agent-config.json, in the agent records insidemanaged-agents.json, and in thecustom_harnesses/*.jsonfiles. This moves every persisted secret out of those JSON files and into the OS keyring, leaving each JSON file holding only an opaque generation reference. The nostr identity keys already lived in the keyring; agent env vars were the missed surface, and custom-harnessenvmaps were a second plaintext surface on the same footing.Both the keyed agent instances and the key-less agent definitions are records in the single unified
managed-agents.json— there is no separateagent-definitions.json; a definition is simply a record with an emptypubkey.Guarantee
In the healthy, keyring-backed state this branch establishes, no provider key or other secret-shaped env value is ever written in plaintext to disk. Each secret lives in the OS keyring under a versioned coordinate; the JSON files carry only an opaque generation reference.
The one deliberate exception is an owner-only inline fallback at
0o600: on a keyless build (system-keyringfeature off) or when a keyring write or its read-back verification fails, the value is kept inline in the owner-only JSON (WriteOutcome::KeptInline) with a warning, and the migration retries on the next boot. The fallback trades the plaintext-at-rest guarantee for availability only when the keyring is genuinely unusable, and only in the app's owner-readable data dir — it never silently degrades a healthy keyring path.How
Gen-ref protocol
Each secret field (
env_vars,auth_tag,provider_config, and harnessenv) gets a companion*_ref: Option<String>holding a generation UUID. On load, when the inline field is empty and a ref is present, the value is hydrated from the keyring. On save, a non-empty inline value is written to the keyring under a versioned coordinate, the inline JSON field is cleared, and the ref is updated to the new generation. Superseded generations are never deleted eagerly — they are retired by a two-cycle GC only once no committed JSON references them.Coordinates are keyed by namespace:
global:env:<gen>,agent:<pubkey>:env|auth_tag|provider_config:<gen>,definition:<slug>:env:<gen>, andharness:<id>:env:<gen>. All coordinates are keys inside the single JSON blob the keyring already uses for identity (servicebuzz-desktop, orbuzz-desktop-devin debug builds; usernamesecrets) — one blob entry, one OS prompt per process, shared with the identity store.Fail-closed tiers
Unavailability — a persisted ref that cannot be hydrated (missing generation, read error, or malformed bytes) — is tracked as an explicit state at every secret-bearing tier, and none silently degrades to an empty env:
ManagedAgentRecord.secrets_unavailable, set when an instance ref fails to hydrate.AgentDefinition.secrets_unavailable, set when a definitionenv_vars_reffails to hydrate.load_global_agent_configreturnsErrwhen a globalenv_vars_refis unresolvable.HarnessDefinition.env_unavailable(a#[serde(skip)]marker, never persisted, reconstructed on load), set byhydrate_harness_envwhen anenv_refis present but hydration fails. Distinct from a genuinely-empty env.Gate topology
There is no single choke point; each consuming boundary refuses independently through the seam appropriate to it.
require_effective_secrets_availableis the effective-config gate for the three agent-secret tiers (global / instance / definition). It is consumed by the config-resolution boundaries — model discovery's config resolve (get_agent_models), persona update, and card mint — and returnsErrwhen any of those three tiers is unavailable.unavailable_definition_id(definition tier) andunavailable_harness_id(harness tier).spawn_agent_child,build_deploy_payload, bothlocal_setupreadiness sites, and model discovery's harness check each consult these predicates and refuse for an unavailable definition or harness.unavailable_harness_idresolves the effective runtime id through the single-sourcedeffective_runtime_id— the same resolver the readiness paths use — so the gate consults exactly the harness a spawn would launch.effective_secrets_unavailable— which ORs the instance (record.secrets_unavailable), definition (unavailable_definition_id), and harness (unavailable_harness_id) tiers — at both the eligibility pre-scan and the under-lock recheck, refusing to select or stop such a record before anystop_managed_agent_process. This closes the stop-then-fail-at-respawn path where an unavailable record's empty hydrated env is indistinguishable from an intentionally-empty one at theagent_readinesslayer, so a live or setup-mode process would otherwise be stopped and only discover the refusal at respawn (FailedAfterStop).refuse_restart_on_unavailable_global, which maps a global-loadErrto a refusal rather thanunwrap_or_default()— at the same restart boundaries: the post-install pre-scan yields zero candidates on a global load error, and both under-lock rechecks refuse before anystop_managed_agent_process(post-install does a strict load; the global-config flow does a strict under-lock re-read of the committed global, since the phase-1 snapshots that drive its readiness comparison cannot attest the ref is still hydratable at stop time).Harness env: ref preservation and rename refusal
strip_harness_envno longer infers a user-clear from an empty env. On a metadata save it takes apreserved_ref(computed bypersisted_unavailable_env_ref, which re-reads the on-disk record with the store directory plumbed in as a parameter): an empty-env save over a persisted-but-unavailable record keeps the rawenv_refrather than erasing it, while a genuine clear of a healthy, fully-hydrated record still clears. This closes the outage-becomes-permanent-pointer-loss path where a transient keyring failure would otherwise be committed as an intentional env clear.Renaming an
env_unavailableharness is refused: becauseharness_env_keyembeds the harness id, carrying a ref forward under a new id would strand it at a coordinate that was never written, leaving the record permanently unavailable even after the keyring recovers. The rename is refused with a clear error until the env re-hydrates or is explicitly re-entered.Commit-point discipline
Old-generation deletion is removed from every save path. The invariant is: write keyring → cancel GC candidacy → clear the inline field → set the ref → commit the JSON atomically. A failed JSON commit still finds the on-disk ref's generation live, because nothing was deleted before the commit landed.
Batched write with generation reuse
A metadata-only save no longer churns the keyring.
write_secrets_batchedpersists env / auth_tag / provider_config in a single blob mutation and, for any field whose bytes byte-equal what the live ref already stores, keeps the existing generation and writes nothing — no new UUID, no mutation, no GC candidacy. Changed fields are staged and committed together in one atomicstore_batch_verified; if that one write fails, all staged fields fall back toKeptInlinetogether, so there is no torn partial state. The boot-migration path keeps its explicitcancel_gc_candidacy; the command path drops it as provably redundant under the transaction lock (a freshly minted or a live-reused generation can never carry a candidate marker at save time).GC exclusion and validation
The full extraction + global save + GC runs under
managed_agents_store_lock; the GC's final JSON read andremove_batchare indivisible against a concurrent save.collect_live_refsvalidates every coordinate before the sweep — malformed, duplicate, or inline+ref-conflict states abort the sweep as a no-op rather than reclaiming a live generation.Cross-process transaction lock
Each
save_managed_agents/save_agent_definitionsacquires a cross-process transaction lock before reading the other half, and holds it across the gen-writes and the atomic JSON commit. Both routes go through one lock-owning seam per surface (save_managed_agents_locked_at/save_agent_definitions_locked_at) that ownslock → raw read → mutate → atomic commit. The identity-persist path takes the same lock through its own lock-owning seam (persist_identity_locked, consumed by import, persist-current, and pairing recover), so an identity save cannot interleave with an agent save against the shared blob. The lock is keyed by the symlink-resolved store directory inode (store_txn_lock_dir), so two Desktop processes sharing one JSON (e.g.just staging+just productionon shared worktrees) cannot interleave a read of one half with the other's write. There is no/tmplock file: the lock is taken directly on the canonical store-directory inode in owner-only app-data (Unixflockon the directory fd; Windows a named kernel mutex viaCreateMutexWderived from the resolved path), so a tmp-cleaner cannot unlink the lock target from under a live transaction. The blob lockfile itself is additionally hardened: after theflockis granted,locked_inode_is_livere-checks that the held fd still refers to the live pathname's inode, catching the classic unlink/recreate split where two processes would otherwise each "hold" the lock over different inodes.Boot migration and scrub
At boot, before the spawn registry warms,
migrate_inline_secrets_to_keyringandmigrate_harness_secrets_to_keyringlift any remaining inline secrets into the keyring, rewrite the JSON stripped at0o600, and — once every projected generation reads back cleanly — scrub the plaintext-bearing backup/temp artifacts (*.json.bak, temp files) the save path can leave behind. Extraction is idempotent across launches: an already-projected file (empty inline + live ref) is re-read and nothing is rewritten. Phase-2 cleanup (legacy scrub/delete) is gated on a verified reload showing zerosecrets_unavailableflags.Dev-service migration
migrate_agent_secrets_to_dev_servicecopies projection keys from the release keyring service to a scoped dev service for standalone worktree launches. The completion marker (_dev_secrets_migration_v2) is written only on a fully clean run; any conflict withholds it so the migration retries next boot. Per-coordinateconflict:<coord>markers are cleared only on proven convergence or proven non-liveness of the coordinate — a source that merely dropped the coordinate does not clear it.Known decisions and limitations
harness:<id>:env:<gen>entries are outsideis_projection_key, so the two-cycle sweep never reclaims them; they accrete slowly in the keyring blob (encrypted at rest — not the plaintext surface this PR closes) on the rare harness-env edit. Follow-up: make custom-harness saves a second consumer ofwrite_secrets_batched, whose gen-reuse makes most accretion never happen.tauri_plugin_single_instance,lib.rs:116) — a duplicate launch is focused into the existing window rather than running a second boot migration.save_*atomic cross-process, but it cannot make a caller's pre-lock snapshot transactional. A command that reads its half under the process-localmanaged_agents_store_lock, mutates, then callssave_*still races a second OS process reading the same JSON before the first commits (last-writer-wins, as wholesale rewrite always was). The gen-ref protocol's new exposure is that a lost write can orphan a just-committed generation. Closing it means promoting ~60save_*call sites to caller-levelload → mutate → committransactions, several spanning.await— beyond this security fix's seam. The seam is marked atacquire_secret_txn_lockinstorage.rs.Test coverage
?propagation on the save paths (no silent inline re-materialization).0o600/ boot-migration coverage including two-launch generation stability, and the harness migration + custom-harness suites.flockEWOULDBLOCKprobe asserting neither half is lost or re-inlined. Mutation-deleting the lock acquisition from each production seam turns the corresponding interleave test red.env_unavailable: hydrate-detection for missing generation, read error, and malformed bytes; the fail-closed predicateunavailable_harness_idbound at the spawn andlocal_setupseams (mutation-checked, with positive controls);strip_harness_envref-preservation on an unavailable record vs. a genuine clear on a healthy one; and rename-while-unavailable refusal with a healthy-rename positive control.Err-path boundaries (refuse_save_on_unavailable_current,resolve_snapshot_global), each with anOkpass-through control and anErr-arm regression. Theget_agent_models, card-mint, and profile-signing boundaries route throughrequire_effective_secrets_available, whose all-tier refusal is already saturated by the effective-config gate tests — no per-command duplicates.